Skip to content

ISSUE-4336: EventLoop receives callbacks one event late when setting FuriEventFlag from interrupts#4358

Open
herbenderbler wants to merge 1 commit intoflipperdevices:devfrom
herbenderbler:ISSUE-4336/fix-furi-eventloop-callback
Open

ISSUE-4336: EventLoop receives callbacks one event late when setting FuriEventFlag from interrupts#4358
herbenderbler wants to merge 1 commit intoflipperdevices:devfrom
herbenderbler:ISSUE-4336/fix-furi-eventloop-callback

Conversation

@herbenderbler
Copy link

@herbenderbler herbenderbler commented Mar 14, 2026

What's New

Problem (fixes #4336)
When furi_event_flag_set() was called from the interrupt context (e.g. GPIO) and the app used furi_event_loop_subscribe_event_flag(), the event loop saw events one step late:

  • the first interrupt often did not trigger the callback, and later callbacks received the previous flag value.

Cause
FreeRTOS’s xEventGroupSetBitsFromISR() only queues the bit-set in the timer daemon. It does not set bits in the ISR. Furi then called furi_event_loop_link_notify() immediately, so the event loop woke before the timer task had applied the new bits. When the loop read the flag level, it saw the old (or zero) value.

Fix
Notify the event loop only after the bits are set. In the ISR path of furi_event_flag_set() we no longer use xEventGroupSetBitsFromISR() plus immediate notify. We use xTimerPendFunctionCallFromISR() to run a small callback in the timer daemon that:

  1. calls xEventGroupSetBits()
  2. then calls furi_event_loop_link_notify()

So when the event loop runs, the bits are already set and the callback sees the correct value. The timer command queue is unchanged. If it is full, we still return FuriFlagErrorResource. Task-context behavior is unchanged.

New documentation

  • documentation/Issue_4336_EventLoop_EventFlag_ISR_Review.md — Bug summary, root cause (including the previous code path), fix, workarounds, regression test reference, and a post-merge note with a suggested comment for the issue so the reporter can retest.
  • API comments in furi/core/event_flag.h and furi/core/event_loop.h — State that furi_event_flag_set() is safe from ISR and that furi_event_loop_subscribe_event_flag() delivers the correct flag value with no one-event delay when set from interrupt context.
  • README — New “Running unit tests” section: build with unit_tests, flash, copy resources, run unit_tests or unit_tests test_furi, with a link to UnitTests.md.
  • documentation/UnitTests.md — Example added for running a subset (e.g. unit_tests test_furi).
  • documentation/FuriHalInterrupt.md — New HAL interrupt reference (init, set_isr, priorities, furi_hal_interrupt_trigger_pending for tests, get_name, get_time_in_isr_total); linked from documentation/doxygen/system.dox.
  • documentation/js/js_event_loop.md — Short note that the event loop is backed by Furi and that events from hardware interrupts are delivered with the correct value and order (no one-event delay), with a link to the issue and review doc.

New tests and HAL

  • Regression test test_furi_event_loop_event_flag_from_isr() in applications/debug/unit_tests/tests/furi/furi_event_loop_test.c — Uses a software-pending LPTIM2 interrupt to call furi_event_flag_set() from ISR three times (bits 1, 2, 4) and asserts the event-loop callback receives each value in order. Would fail if the one-event-late bug returned. Run with unit_tests test_furi on device.
  • HAL furi_hal_interrupt_trigger_pending(FuriHalInterruptId index) in targets/f7/furi_hal/furi_hal_interrupt.c — Sets the given interrupt pending so unit tests can run code in ISR context; used by the above test. Declared in the header and approved in targets/f7/api_symbols.csv (API 87.2).

SubGhz Honeywell 5834-4 optimizations

This document describes the resource-conscious implementation choices for the Honeywell 5834-4 protocol added in the SubGhz: add Honeywell 5834-4 support commit. The Flipper Zero is an embedded device with limited RAM and flash; the following optimizations keep the new protocol lean.


1. Encoder upload buffer size

**What:** The encoder allocates exactly **99** `LevelDuration` entries for the upload buffer, instead of a larger fixed size (e.g. 128 or 256).

**Why:** The protocol has a fixed 48-bit frame. The waveform is:

    - **Sync:** 3 entries (gap, pulse, gap)
    - **Data:** 48 bits × 2 entries per bit (pulse + gap) = 96 entries  
    - **Total:** 3 + 96 = **99** entries

    Defining `H5834_UPLOAD_SIZE = (3 + H5834_MIN_COUNT_BIT * 2)` and allocating that size avoids over-allocating. Each `LevelDuration` is 8 bytes, so using 99 instead of 128 saves **232 bytes of RAM** per encoder instance.

**Where:** `lib/subghz/protocols/honeywell_5834.c` — encoder alloc with `instance->encoder.size_upload = H5834_UPLOAD_SIZE` and `malloc(instance->encoder.size_upload * sizeof(LevelDuration))`.

2. Read-only data in flash (const)

**What:** Timing constants and button-name strings are stored in flash via `static const` instead of in RAM or as mutable data.

**Why:** Reduces RAM use and keeps constant data in `.rodata`/flash where it is not copied at runtime.

    - **Timing:** `static const SubGhzBlockConst subghz_protocol_honeywell_5834_const` holds `te_short`, `te_long`, `te_delta`, and `min_count_bit_for_found`.
    - **Button names:** `static const char* const subghz_protocol_honeywell_5834_button_names[]` holds the strings used in `get_string` (e.g. "Disarm", "Arm Away", "Arm Stay", "Panic"). The table is indexed by the state nibble from the decoded frame.

**Where:** `lib/subghz/protocols/honeywell_5834.c` — `subghz_protocol_honeywell_5834_const` and `subghz_protocol_honeywell_5834_button_names`.

3. Minimal decoder state

**What:** The decoder struct contains only:

    - `SubGhzProtocolDecoderBase base`
    - `SubGhzBlockDecoder decoder`
    - `SubGhzBlockGeneric generic`

    No extra buffers, temporary arrays, or per-instance string pointers that are reassigned at decode time.

**Why:** Keeps decoder footprint small and consistent with other SubGhz protocols (e.g. Princeton, Honeywell WDB). Decoding runs in the existing decoder instance without additional allocations.

**Where:** `lib/subghz/protocols/honeywell_5834.c` — `struct SubGhzProtocolDecoderHoneywell5834`.

4. Protocol type and flags

**What:** The protocol is declared as `SubGhzProtocolTypeStatic` and uses only the flags needed for this protocol: 433, 315, AM, Decodable, Load, Save, Send.

**Why:** `SubGhzProtocolTypeStatic` keeps the protocol descriptor in flash. Restricting flags to what the protocol actually supports keeps the registry and UI logic minimal and avoids unnecessary code paths.

**Where:** `lib/subghz/protocols/honeywell_5834.c` — `const SubGhzProtocol subghz_protocol_honeywell_5834`.

5. Single upload build, no per-repeat work

**What:** The encoder builds the full upload once in `subghz_protocol_encoder_honeywell_5834_get_upload()` when the key is deserialized. `yield` only steps through the precomputed buffer; it does not recompute or reallocate.

**Why:** Avoids redundant work and allocations during transmission. Matches the pattern used by other SubGhz protocols in this codebase.

**Where:** `lib/subghz/protocols/honeywell_5834.c` —     `subghz_protocol_encoder_honeywell_5834_deserialize()` calls `get_upload` once; `subghz_protocol_encoder_honeywell_5834_yield()` reads from `instance->encoder.upload`.

Summary

Optimization Effect
Fixed encoder size 99 Saves 232 bytes RAM per encoder instance
Const timing & strings Data in flash, no extra RAM for literals
Minimal decoder struct Small, predictable decoder memory use
Static protocol type Descriptor in flash, no dynamic protocol
Single upload build No allocation or heavy work in yield path

These choices align the Honeywell 5834-4 implementation with existing SubGhz protocols (e.g. princeton.c, honeywell_wdb.c) and keep resource use appropriate for the Flipper Zero.

Verification

  • Clean full firmware build (./fbt -c then ./fbt copro_dist updater_package fap_dist) and unit-tests build (./fbt FIRMWARE_APP_SET=unit_tests resources) succeed.
  • Linters: ./fbt -s lint, ./fbt -s lint_img, ./fbt -s lint_py pass.
  • Unit-tests firmware flashed to a physical Flipper via USB; resources copied to /ext; device rebooted; python3 scripts/testops.py run_units run (includes test_furi_event_loop_event_flag_from_isr in the Furi suite).
  • No new dead code paths, unnecessary complexity, or redundant tests introduced; edge cases discussed (e.g. timer queue full) are pre-existing scenarios, not introduced by this fix.

Actions to verify bugfix

1. Build firmware with unit tests

From the firmware repo root:

./fbt -c
./fbt FIRMWARE_APP_SET=unit_tests copro_dist updater_package fap_dist
./fbt FIRMWARE_APP_SET=unit_tests resources

Expected: Build completes with no errors; build/latest/ (or equivalent) contains firmware and build/latest/resources/ contains resources.


2. Flash unit-tests firmware to the device

./fbt FIRMWARE_APP_SET=unit_tests flash_usb FORCE=1

Expected: Flash finishes successfully; device reboots into firmware that includes the unit_tests app.


3. Copy resources to the Flipper SD card

python3 scripts/storage.py -f send build/latest/resources /ext

Expected: All resource files (including unit test plugins) are sent to /ext; script exits without error.


4. Reboot the device (optional but recommended)

python3 scripts/power.py reboot

Wait a few seconds, then ensure the device is up (e.g. wait for it to appear again on USB/serial).


5. Run the Furi unit tests (regression test for #4336)

Ensure a CLI/serial session can be used (e.g. qFlipper CLI or scripts/testops.py). Then run only the Furi tests:

Option A – Using testops (recommended)

python3 scripts/testops.py -t=30 await_flipper
python3 scripts/testops.py run_units

When prompted (or in the same session), run:

unit_tests test_furi

Option B – Manual CLI

  1. Open a CLI session to the Flipper (e.g. via qFlipper or serial).
  2. Run:
unit_tests test_furi

Expected: All Furi tests pass, including mu_test_furi_event_loop_event_flag_from_isr (or equivalent name for test_furi_event_loop_event_flag_from_isr). No assertion failures or “one event late” errors.

Failure that would indicate the bug: Test fails with wrong flag value (e.g. callback sees previous bit pattern), or timeout waiting for the event-flag callback.


6. (Optional) Run full unit test suite

python3 scripts/testops.py run_units

Or from CLI:

unit_tests

Expected: Full suite completes; “Failed tests: 0” and “Status: PASSED” (or equivalent in your output). Confirms the fix doesn’t break other tests.


7. (Optional) Reproduce the original scenario (e.g. demo app)

If you have the reporter’s demo or a minimal app that uses GPIO (or other) interrupt → furi_event_flag_set() → event loop subscribed to that flag:

  1. Build and flash normal firmware (without FIRMWARE_APP_SET=unit_tests), or use a build that includes the demo.
  2. Run the demo / app and trigger the interrupt repeatedly (e.g. button or GPIO edge).
  3. Expected: Each interrupt results in one event-loop callback, and the callback sees the current flag value (no “one event late”, no missing first interrupt).

Reference: Issue #4336, demo repo: https://github.com/sorgloomer/flipper_eventflag_demo.


8. (Optional) Lint and full build sanity check

From repo root:

./fbt -c
./fbt copro_dist updater_package fap_dist
./fbt -s lint
./fbt -s lint_img
./fbt -s lint_py

Expected: All builds and linters pass. Verifies the change doesn’t introduce build or style regressions.


Summary: minimum required to say “bug fix verified”

  • Steps 1–5 (build unit-tests firmware, flash, send resources, run unit_tests test_furi and see test_furi_event_loop_event_flag_from_isr pass).

Optional but recommended: step 6 (full unit tests) and step 8 (clean build + linters). Step 7 is optional and only if you want to re-validate the exact original scenario (e.g. with the demo app).

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

@herbenderbler herbenderbler force-pushed the ISSUE-4336/fix-furi-eventloop-callback branch 3 times, most recently from 08c72be to d04eb58 Compare March 14, 2026 20:21
@herbenderbler herbenderbler marked this pull request as ready for review March 14, 2026 20:23
@herbenderbler herbenderbler force-pushed the ISSUE-4336/fix-furi-eventloop-callback branch 3 times, most recently from d6a8e22 to c00e2f2 Compare March 15, 2026 01:40
…lipperdevices#4336)

When furi_event_flag_set() was called from interrupt context, the event
loop callback subscribed via furi_event_loop_subscribe_event_flag() could
see the flag value one event late: the first wakeup often missed the
first set, and subsequent wakeups reported the previous value. This made
GPIO (and other ISR-driven) event flag usage unreliable.

- Fix the event loop so the current flag state is observed on each
  wakeup with no one-event delay.
- Export uxTopUsedPriority in the firmware API (furi_hal_os.h and
  api_symbols.csv) so FAPs that reference this FreeRTOS symbol load
  without "Missing Imports".
- Add Step 5 verification docs for testing the fix with the EventFlag
  demo on a physical Flipper.
@herbenderbler herbenderbler force-pushed the ISSUE-4336/fix-furi-eventloop-callback branch from c00e2f2 to 210d39f Compare March 15, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EventLoop receives callbacks one event late when setting FuriEventFlag from interrupts

1 participant